refactor: PlatformRegistry pattern#18
Conversation
| export interface PlatformDependencies { | ||
| TimeCoordinator: TimeCoordinator; | ||
| DPoPSigningAuthority: DPoPSigningAuthority; | ||
| } |
There was a problem hiding this comment.
looking ahead, just curious if is there anything currently in the plan to extend this and support any specific additional global dependencies? i.e. there any scope for telemetry or a logger for errors or certain oauth operations?
There was a problem hiding this comment.
Both Telemetry and Loggers are great examples of things we could put in here. There probably won't be specific OAuth operations, but perhaps singletons to enable OAuth operations, like the DPoPSigningAuthority for example.
I will probably add things here I expect to get implemented at a platform level, like TokenStorage. The advantage is the default In-Memory Token storage impl won't be included in the bundle
| * For example, the {@link TimeCoordinator} should be globally available to be a | ||
| * centralized entity to perform all time calculations. Registering the {@link TimeCoordinator} | ||
| * as a {@link Platform} dependency enables consumers to access the {@link TimeCoordinator} via | ||
| * | ||
| * @example | ||
| * ``` | ||
| * import { Platform } from '@okta/auth-foundation'; | ||
| * const currentTime = Platform.TimeCoordinator.now(); | ||
| * ``` | ||
| * | ||
| * To enable tree-shaking and prevent including default implementations (bundle bloat) which | ||
| * will be instanceously overwritten, default implemenations can be selectively included. | ||
| * | ||
| * @remarks | ||
| * Use `import * from '@okta/auth-foundation'` for standard usage, including all default platform | ||
| * dependency implementations. | ||
| * | ||
| * Use `import * from '@okta/auth-foundation/core'` for deeper customizations of platform dependencies, | ||
| * this does not include any default implementations. {@link PlatformRegistryError} will be thrown if | ||
| * a dependency is used before an implementation is provided | ||
| */ |
There was a problem hiding this comment.
the usage is very clearly explained here
| /** | ||
| * A singleton registry of globally-available singleton dependencies which can | ||
| * provide platform-specific default implementations and enable overriding as needed | ||
| * |
There was a problem hiding this comment.
the first time i read the class name PlatformRegistry, i assumed more of a registry of platforms i.e. browsers/node rather than a registry of dependencies that can be overriden to support different platforms. i can't come up with a better name myself but just wanted to give feedback in case you had a better name in mind
There was a problem hiding this comment.
Naming is hard
| * ``` | ||
| * | ||
| * To enable tree-shaking and prevent including default implementations (bundle bloat) which | ||
| * will be instanceously overwritten, default implemenations can be selectively included. |
|
|
||
| /** @internal */ | ||
| export const DefaultDPoPSigningAuthority: DPoPSigningAuthority = new DPoPSigningAuthorityImpl(new DPoPStorage.MemoryStore()); | ||
| /** @internal - Use `Platform.DPoPSigninAuthority instead */ |
| * import { Platform } from '@okta/auth-foundation/core'; // ensure "/core" is imported specifically | ||
| * | ||
| * Platform.registerDefaultsLoader(() => ({ | ||
| * TimeCoordinator: MyCustomTimeCoordinator |
There was a problem hiding this comment.
does this need to include all of the PlatformDependencies i.e. including the DPoPSigningAuthority and DPoPNonceCache?
There was a problem hiding this comment.
Good catch, I guess the doc comments are clear 😅
There was a problem hiding this comment.
I added the comment in response to your feedback. "guess the doc comments are clear" meant, you clearly understand the dependencies/required inputs (otherwise you wouldn't have caught that)
3 participants
No description provided.